Freshdesk - add support for notes #17598
Conversation
|
@seynadio is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe update assigns a default ticket name in the update-ticket action, adds a new action to add notes to Freshdesk tickets with input validation, and introduces a new method Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AddNoteAction
participant FreshdeskApp
participant FreshdeskAPI
User->>AddNoteAction: Invoke add-note-to-ticket with props
AddNoteAction->>FreshdeskApp: addNoteToTicket(ticketId, data)
FreshdeskApp->>FreshdeskAPI: POST /tickets/{ticketId}/notes with data
FreshdeskAPI-->>FreshdeskApp: Note creation response
FreshdeskApp-->>AddNoteAction: Return response
AddNoteAction-->>User: Return summary and API response
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/freshdesk/actions/update-ticket/update-ticket.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs components/freshdesk/freshdesk.app.mjsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'jsonc-eslint-parser' imported from /eslint.config.mjs 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs (1)
🔇 Additional comments (7)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
|
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/freshdesk/actions/update-ticket/update-ticket.mjs (1)
74-86: Properties look good but consider validation enhancement.The new properties are well-defined with clear descriptions and appropriate types.
Consider adding validation to ensure
noteBodyis provided wheninternalNoteis true:noteBody: { type: "string", label: "Note Body", description: "The content of the internal note to add.", optional: true, + reloadProps: true, },Or add a conditional requirement in the property definition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
components/freshdesk/actions/update-ticket/update-ticket.mjs(3 hunks)
🔇 Additional comments (3)
components/freshdesk/actions/update-ticket/update-ticket.mjs (3)
8-8: LGTM: Version increment is appropriate.The version bump from 0.0.1 to 0.0.2 correctly reflects the addition of new functionality.
111-112: LGTM: Destructuring correctly extracts new properties.The destructuring properly extracts the new properties while maintaining the existing
...fieldspattern.
135-149: LGTM: Existing logic preserved correctly.The existing ticket update logic remains intact and functions as expected when not adding internal notes.
| if (internalNote && noteBody) { | ||
| const response = await freshdesk._makeRequest({ | ||
| $, | ||
| method: "POST", | ||
| url: `/tickets/${ticketId}/notes`, | ||
| data: { | ||
| body: noteBody, | ||
| private: true, | ||
| }, | ||
| }); | ||
|
|
||
| $.export("$summary", `Internal note added to ticket "${ticketName}" (ID: ${ticketId})`); | ||
| return response; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address the either/or behavior limitation and validation gap.
The current implementation has two design issues:
-
Either/or limitation: Users can either add an internal note OR update ticket fields, but not both in the same action call. This may not meet user expectations.
-
Validation gap: If
internalNoteis true butnoteBodyis empty/falsy, the function proceeds to regular update logic, potentially throwing a confusing "Please provide at least one field to update" error.
Consider these improvements:
Option 1: Allow both operations in sequence
if (internalNote && noteBody) {
- const response = await freshdesk._makeRequest({
+ await freshdesk._makeRequest({
$,
method: "POST",
url: `/tickets/${ticketId}/notes`,
data: {
body: noteBody,
private: true,
},
});
-
- $.export("$summary", `Internal note added to ticket "${ticketName}" (ID: ${ticketId})`);
- return response;
+ }
+
+ if (internalNote && !noteBody) {
+ throw new Error("Note Body is required when Internal note is enabled.");
}Option 2: Add validation for the either/or scenario
+ if (internalNote && !noteBody) {
+ throw new Error("Note Body is required when Internal note is enabled.");
+ }
+
if (internalNote && noteBody) {
// existing logic
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (internalNote && noteBody) { | |
| const response = await freshdesk._makeRequest({ | |
| $, | |
| method: "POST", | |
| url: `/tickets/${ticketId}/notes`, | |
| data: { | |
| body: noteBody, | |
| private: true, | |
| }, | |
| }); | |
| $.export("$summary", `Internal note added to ticket "${ticketName}" (ID: ${ticketId})`); | |
| return response; | |
| } | |
| if (internalNote && noteBody) { | |
| await freshdesk._makeRequest({ | |
| $, | |
| method: "POST", | |
| url: `/tickets/${ticketId}/notes`, | |
| data: { | |
| body: noteBody, | |
| private: true, | |
| }, | |
| }); | |
| } | |
| if (internalNote && !noteBody) { | |
| throw new Error("Note Body is required when Internal note is enabled."); | |
| } |
🤖 Prompt for AI Agents
In components/freshdesk/actions/update-ticket/update-ticket.mjs around lines 120
to 133, the code currently only allows adding an internal note or updating
ticket fields, not both, and lacks validation when internalNote is true but
noteBody is empty. To fix this, modify the logic to support performing both
actions sequentially if both internalNote and ticket updates are provided, or
alternatively, add validation to check if internalNote is true but noteBody is
missing and return a clear error before proceeding. This ensures users can
update tickets and add notes in one call or receive proper feedback if required
inputs are missing.
- Created dedicated add-note-to-ticket action for better separation of concerns - Removed note functionality from update-ticket action to maintain single responsibility - Added addNoteToTicket method to freshdesk.app.mjs - Bumped update-ticket version to 0.0.3 This provides a cleaner API where each action has a clear, single purpose. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs (2)
78-80: Ensure type safety for user_id parameter.The explicit conversion to Number is good practice, but consider adding validation to ensure the conversion is successful.
Apply this diff to add validation:
if (user_id) { - data.user_id = Number(user_id); + const userId = Number(user_id); + if (isNaN(userId)) { + throw new ConfigurationError("User ID must be a valid number"); + } + data.user_id = userId; }
82-84: Add validation for notify_emails array.Consider validating that the email addresses are properly formatted to prevent API errors.
Apply this diff to add email validation:
if (notify_emails && notify_emails.length > 0) { + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + const invalidEmails = notify_emails.filter(email => !emailRegex.test(email)); + if (invalidEmails.length > 0) { + throw new ConfigurationError(`Invalid email addresses: ${invalidEmails.join(", ")}`); + } data.notify_emails = notify_emails; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs(1 hunks)components/freshdesk/actions/update-ticket/update-ticket.mjs(1 hunks)components/freshdesk/freshdesk.app.mjs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- components/freshdesk/actions/update-ticket/update-ticket.mjs
🔇 Additional comments (4)
components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs (4)
4-9: LGTM! Action metadata follows conventions.The action key follows the required naming convention (
app_name_slug-slugified-component-name) and includes proper metadata fields.
10-51: Props definition looks comprehensive and well-structured.The props are well-defined with appropriate types, labels, descriptions, and optional flags. Good use of propDefinitions for reusable props like
ticketIdandagentId.
63-65: Input validation is appropriate.The validation ensures the note body is not empty, which prevents unnecessary API calls with invalid data.
86-94: API call implementation and response handling look correct.The implementation properly passes the context object ($), converts ticketId to Number for type safety, and provides a meaningful summary message.
| throw new ConfigurationError("Note body cannot be empty"); | ||
| } | ||
|
|
||
| const ticketName = await freshdesk.getTicketName(ticketId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify error handling for getTicketName method.
Ensure that if the ticket doesn't exist, the error is handled gracefully.
Run the following script to check if getTicketName has proper error handling:
🏁 Script executed:
#!/bin/bash
# Check if getTicketName method has error handling for non-existent tickets
ast-grep --pattern $'async getTicketName($TICKET_ID) {
$$$
}'Length of output: 64
To locate the getTicketName implementation and inspect its error handling, let’s search for its definition across the codebase:
🏁 Script executed:
#!/bin/bash
# Find all occurrences of getTicketName to locate its implementation
rg -n "getTicketName" --color=neverLength of output: 989
Let’s inspect the implementation around line 253 to see how errors are handled:
🏁 Script executed:
#!/bin/bash
# Show getTicketName implementation with context
rg -n "async getTicketName" -A15 -B5 components/freshdesk/freshdesk.app.mjsLength of output: 612
To verify how missing tickets are handled, let’s inspect the getTicket implementation:
🏁 Script executed:
#!/bin/bash
# Show getTicket implementation with context to check error handling
rg -n "async getTicket" -A15 -B5 components/freshdesk/freshdesk.app.mjsLength of output: 1134
Let’s inspect the _makeRequest implementation to see how it handles HTTP errors (e.g., 404 for non-existent tickets):
🏁 Script executed:
#!/bin/bash
# Locate _makeRequest implementation with surrounding context
rg -n "async _makeRequest" -A20 -B5 components/freshdesk/freshdesk.app.mjsLength of output: 805
Add explicit error handling for missing tickets in getTicketName
The getTicketName helper currently propagates any errors (including 404s) up to the caller. We need to catch “ticket not found” errors and handle them gracefully—either by returning null (or a default) or by surfacing a clear, user-friendly error.
Affected locations:
- components/freshdesk/freshdesk.app.mjs at
getTicketName(around line 253) - components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs at the call site (line 67)
Proposed changes:
- Wrap
getTicketNameimplementation in a try/catch and handle HTTP 404:
--- a/components/freshdesk/freshdesk.app.mjs
+++ b/components/freshdesk/freshdesk.app.mjs
@@ -251,7 +251,15 @@
async getTicketName(ticketId) {
- const ticket = await this.getTicket({ ticketId });
- return ticket.subject;
+ try {
+ const ticket = await this.getTicket({ ticketId });
+ return ticket.subject;
+ } catch (error) {
+ if (error.response?.status === 404) {
+ // Ticket not found; return null or a default placeholder
+ return null;
+ }
+ // Re-throw other errors
+ throw error;
+ }
},- Handle the
null(or fallback) at the action layer so the flow doesn’t break unexpectedly:
--- a/components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs
+++ b/components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs
@@ -64,7 +64,13 @@
const ticketId = await freshdesk.searchTickets({ /* … */ });
- const ticketName = await freshdesk.getTicketName(ticketId);
+ let ticketName;
+ try {
+ ticketName = await freshdesk.getTicketName(ticketId);
+ if (!ticketName) {
+ // Optional: handle missing ticket (e.g., log warning or set default)
+ ticketName = "Unknown Ticket";
+ }
+ } catch (error) {
+ throw new Error(`Failed to fetch ticket name: ${error.message}`);
+ }
// … proceed with adding the noteThis ensures that 404s don’t crash your action and that any other errors bubble up clearly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ticketName = await freshdesk.getTicketName(ticketId); | |
| async getTicketName(ticketId) { | |
| try { | |
| const ticket = await this.getTicket({ ticketId }); | |
| return ticket.subject; | |
| } catch (error) { | |
| if (error.response?.status === 404) { | |
| // Ticket not found; return null or a default placeholder | |
| return null; | |
| } | |
| // Re-throw other errors | |
| throw error; | |
| } | |
| }, |
| const ticketName = await freshdesk.getTicketName(ticketId); | |
| const ticketId = await freshdesk.searchTickets({ /* … */ }); | |
| let ticketName; | |
| try { | |
| ticketName = await freshdesk.getTicketName(ticketId); | |
| if (!ticketName) { | |
| // Optional: handle missing ticket (e.g., log warning or set default) | |
| ticketName = "Unknown Ticket"; | |
| } | |
| } catch (error) { | |
| throw new Error(`Failed to fetch ticket name: ${error.message}`); | |
| } | |
| // … proceed with adding the note |
🤖 Prompt for AI Agents
In components/freshdesk/actions/add-note-to-ticket/add-note-to-ticket.mjs at
line 67, the call to getTicketName does not handle the case where the ticket is
missing (HTTP 404). Update the getTicketName function in
components/freshdesk/freshdesk.app.mjs around line 253 to wrap its logic in a
try/catch block that catches 404 errors and returns null or a default value
instead of throwing. Then, at the call site in add-note-to-ticket.mjs line 67,
check if the returned ticketName is null or fallback and handle that case
gracefully to prevent the action from breaking unexpectedly.
- Add JSDoc documentation to addNoteToTicket method - Add error handling for missing tickets in getTicketName (returns null for 404) - Add validation for user_id parameter to ensure valid number - Add email validation for notify_emails array - Update ticket name handling to use fallback when null 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
I've addressed all the code review comments from CodeRabbit:
All changes have been tested and committed. The PR is ready for review. |
WHY
New Features
Chores
Summary by CodeRabbit